Skip to content

feat(acp,desktop): resume interrupted turns after app restart - #2177

Closed
wpfleger96 wants to merge 1 commit into
mainfrom
duncan/acp-resume-ledger
Closed

feat(acp,desktop): resume interrupted turns after app restart#2177
wpfleger96 wants to merge 1 commit into
mainfrom
duncan/acp-resume-ledger

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Jul 20, 2026

Copy link
Copy Markdown
Member

Summary

Managed agents lose in-flight work when the app restarts mid-turn. This adds a durable pending-turn ledger to the buzz-acp harness so an interrupted turn is replayed on the next boot, plus a per-agent Desktop toggle so the behavior can be turned off for a given agent.

Harness (crates/buzz-acp)

  • ledger.rs mirrors the in-memory EventQueue to disk after every queue mutation, persisting through a temp file plus rename.
  • Boot recovery in lib.rs: staged ledger load → membership gate → TTL expiry → chunked REST fetch with per-event reconciliation (signature/id/channel verification) → ordered queue import → unresolved ordering-barrier registration → one atomic commit.
  • A dedicated tokio::time::Sleep arm in the main select! loop fires at the earliest unresolved barrier deadline, so the barrier timeout is a real bound even on a completely quiet harness.
  • Live-admission seam: recovered events resolved over WebSocket bypass the steer/interrupt path and dispatch with recovery framing; a suppression set prevents duplicate processing across boot fetch and live delivery.
  • Shutdown-drain completion classifies results through complete_batch + ledger sync at both grace-period drain sites; the join-error arm applies a queue-only panic disposition.
  • queue.rs gains complete_batch (atomic payload-owning completion), shared enforce_cap, import_recovered/admit_recovered, set_unresolved_barrier/next_unresolved_barrier_deadline, and dirty_channels tracking.
  • Membership removal purges the removed channel's durable state completely: drain_channel drops the in-flight batch's recovery mirror alongside the queued events, so the trailing ledger sync cannot re-persist a trigger Ledger::invalidate_channel just purged. in_flight_channels/in_flight_deadlines stay preserved so the live task still completes or expires.
  • config.rs gains --resume-on-restart / --resume-ttl-secs, with BUZZ_ACP_RESUME_ON_RESTART as the env equivalent. Resume is on by default.
  • format_prompt emits a recovery header and per-event [restart recovery] markers for recovered batches.
  • The windows-rust CI job runs cargo test -p buzz-acp ledger::, giving the temp-file-plus-rename persist path Windows-native coverage instead of relying on POSIX rename semantics.
  • When resume_on_restart is false, boot best-effort deletes the ledger file so toggling off truly drops pending work — a future re-enable starts clean.

Desktop toggle

  • ManagedAgentRecord.resume_on_restart defaults to true via serde, so existing stores deserialize unchanged and no migration is needed.
  • BUZZ_ACP_RESUME_ON_RESTART is reserved (strips from user env-var editor and on-disk records) and emitted unconditionally from spawn so the UI toggle is the sole control plane in both directions — no ambient parent-process leak when ON, no user env override when OFF.
  • The field participates in spawn_hash, so flipping it raises the restart-required badge rather than leaving a stale process running the old setting.
  • "Resume interrupted turns" switch in the Advanced section of the agent Edit dialog, persisted through the standalone set_managed_agent_resume_on_restart command in the same shape as the auto-restart toggle.

Dialog routing fix

The agent-profile pencil path (handleEditAgent) previously short-circuited to the definition dialog for any definition-backed agent, making all instance-level settings unreachable from the UI for agents created via the create flow. The pencil now unconditionally opens the instance-edit dialog, which already handles linked agents (inherited values shown, "edit linked definition" escape hatch wired). The separate handleEditPersona callback retains the definition-edit surface for surfaces that explicitly target it.

Comment blocks in the touched desktop files are condensed so each file stays within the differential file-size ratchet.

Deferred follow-ups

These were raised as MINOR findings and are non-blocking per reviewer sign-off. Each has a clear fix shape and bounded scope.

  • Boot recovery gated on membership, not subscription scope (CLI-only). When multiple --channels are passed, boot_recover only attempts recovery for channels whose membership the harness fetched at startup. A channel added to the CLI args but not yet joined (e.g. a new invite) may have ledger records that silently skip recovery. Fix shape: intersect the staged ledger's channel set with the --channels subscription set rather than the membership set. Does not affect the Desktop path, which always passes the full subscribed channel list.
  • Permanently unfetchable unresolved records re-arm the 60s ordering barrier each boot. If a ledger entry's event can never be fetched (deleted from relay, bad signature), it stays in the unresolved set indefinitely and the ordering barrier fires on every restart. Fix shape: track per-record fetch-attempt count; after a configurable cap (e.g. 3), classify the record as permanently unresolvable and drop it from the ledger.

@wpfleger96
wpfleger96 requested a review from a team as a code owner July 20, 2026 16:29
@wpfleger96
wpfleger96 force-pushed the duncan/acp-resume-ledger branch from 8d72643 to a17863c Compare July 20, 2026 16:54
@wpfleger96
wpfleger96 marked this pull request as draft July 20, 2026 16:57
Comment thread .github/workflows/signed-macos-canary.yml Fixed
@wpfleger96
wpfleger96 marked this pull request as ready for review July 28, 2026 18:59
@wesbillman

Copy link
Copy Markdown
Collaborator

Thanks for the substantial work on recovery and persistence here. I reviewed the current head (82520f6fbb0a68307e15e32bf40abe78827713c0) and found two correctness issues that need to be fixed before this is safe to merge:

1. Membership removal can re-persist an in-flight batch after invalidation

In crates/buzz-acp/src/lib.rs, the membership-removal path drains the channel and calls ledger.invalidate_channel(ch), but the later sync_dirty can write the channel back immediately:

  • drain_channel deliberately preserves the channel's in-flight state.
  • recoverable_triggers includes in_flight_batch_triggers.
  • The subsequent ledger sync therefore includes the trigger that was just invalidated.

If the harness crashes before that prompt completes, a later boot after the agent is re-added can recover work that membership removal intended to purge.

Please make invalidation win over the dirty sync—for example, consume/sync the dirty queue state first and perform channel invalidation as the final persistence operation, or introduce a channel-removal transition that excludes/removes that channel's in-flight recovery triggers.

Please also add a regression test that dispatches a batch, removes membership while the batch is in flight, immediately reloads the ledger, and verifies that the channel is absent.

2. Replacing an existing ledger is not portable to Windows

crates/buzz-acp/src/ledger.rs currently persists through a fixed temporary sibling followed by:

std::fs::rename(&tmp_path, path)

On Windows, std::fs::rename fails when the destination already exists. The first persist creates the ledger, but later updates cannot replace it, leaving stale recovery state on disk.

Please use a cross-platform atomic-replacement implementation. Deleting the destination before renaming would not be sufficient because it creates a crash window with no ledger.

Please add Windows-executed coverage that persists snapshot A, persists a changed snapshot B to the same path, reloads it, and observes B. The current Windows CI lane compiles buzz-acp but does not execute this filesystem behavior.

The delta since the originally reviewed head 53392cc55e31e251aceb96ea33aedd51520e9ec3 only deletes tests; production code is unchanged, so both issues remain at the current head. The branch is also currently conflicting with main. Once the two fixes and discriminating regression tests are in place, please refresh from main and run fresh CI; the follow-up review can then focus on that delta.

@wpfleger96
wpfleger96 force-pushed the duncan/acp-resume-ledger branch from 82520f6 to 9ba32db Compare July 28, 2026 21:59
@cameronhotchkies cameronhotchkies added the triage-ready Appropriate for agentic review label Jul 30, 2026
@wpfleger96 wpfleger96 changed the title feat(buzz-acp): durable pending-turn ledger for agent auto-resume after app restart feat(acp,desktop): resume interrupted turns after app restart Jul 30, 2026
@wpfleger96
wpfleger96 force-pushed the duncan/acp-resume-ledger branch 2 times, most recently from 5bae077 to 9185a75 Compare August 2, 2026 17:22
Managed agents lost any in-flight or queued turn when the app restarted:
the event queue lived only in memory, so a mention being worked on at
shutdown was silently dropped and never answered.

Adds `ledger.rs`, a durable pending-turn mirror written after every queue
mutation, and a boot-recovery pipeline in `lib.rs`: staged load, membership
gate, TTL expiry, chunked REST fetch with per-event signature/id/channel
reconciliation, ordered queue import, unresolved ordering barrier, then one
atomic commit. Recovered turns re-run whole (there is no mid-turn
checkpoint) and carry a `[restart recovery]` marker so the agent re-reads
the thread before redoing work.

Exactly-once is enforced on two fronts: a suppression set keyed by event id
prevents the boot fetch and live WebSocket delivery from both admitting the
same event, and events whose REST fetch failed stay unresolved behind an
ordering barrier with a real deadline — a dedicated timer arm in the main
`select!` loop fires it even on a completely quiet harness, so the timeout
is a bound rather than a hope.

`queue.rs` gains payload-owning `complete_batch`, shared `enforce_cap`,
`import_recovered`/`admit_recovered`, barrier registration and
`dirty_channels` tracking; shutdown-drain now classifies results through
`complete_batch` plus a ledger sync at both grace-period sites so a turn
completed during drain is not resurrected on the next boot. Off-by-default
paths are opt-out via `--resume-on-restart`/`--resume-ttl-secs`.

Desktop: adds per-agent resume-on-restart toggle (Advanced settings), makes
the toggle authoritative over env precedence (reserved key), and guarantees
the ledger file is deleted on every boot where resume is off. Routing fix:
handleEditAgent now opens the instance editor for started agents, definition
editor for unstarted ones.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 force-pushed the duncan/acp-resume-ledger branch from 9185a75 to 9c5efca Compare August 3, 2026 15:44
@wpfleger96

Copy link
Copy Markdown
Member Author

closing for now after discussing with the team

@wpfleger96 wpfleger96 closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-ready Appropriate for agentic review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants